ci: Use ASan for unit tests#98
Conversation
|
Is there a Duolingo course for GitHub, where you have to open a PR everyday, so that the green bird doesn't torture your family? Now, with that detour out of the way. I feel like the fixed-point code is doing much more then the UBSan can comprehend with the Looks good to me, like you said - the UBSan can be added later. |
Just doing anything at all to procrastinate writing my thesis I guess
Apart from bugs, afaik UBSan generally doesn't really have false positives so I'd still be suspicious
Not sure about the behavior of these specific errors, but generally with ub it's possible that even if it's working fine today in some specific configuration, the results could change in the future due to optimization level, different compiler, compiler update, different architecture or some seemingly innocent, unrelated code change. That can make bugs hard to track down, so it would be nice to eventually have no ubsan errors, or be really sure these ones are fine. It might not be that bad though. I noticed that relaxing the integer overflow rules with |
haha, I feel you!
Yea, that's fair. I'll look into the exact input values that cause these errors.
Yea, I get it. What I'm most curious about is how this: YeetMouse/driver/FixedMath/Fixed64.h Lines 372 to 376 in 528747c does not cause any ub / errors. It works for negative and positive numbers, up to The current UB errors are mostly caused by the trigonometric functions (and their subcalls). I'll look into that later, but after a quick glance these errors are mostly explained by the comments in the code.
Yea, that's interesting. Kind of makes sense, as all these UBs are just shifting the number out of bounds to get the fractional part or something like that. Like I said before, I think most (if not all) of these could be fixed by casting the number to the unsigned-version of its type. |
|
I actually prompted a clanker from github to fix these issues, and it gave up after like 2 minutes of adding (I reached the token limit with that one prompt, even though I thought I have github Pro...) |
Use Address Sanitizer for unit tests in the CI to help catch memory related mistakes. Undefined Behavior Sanitizer (UBSan) should also be enabled eventually, however the unit tests currently have some ub related to left shfiting negative integers and signed integer overflow in the fixed point arithmetic code, which should be looked at first.